Skip to content

Correction for short function syntax, closes #356#359

Merged
hsutter merged 1 commit intohsutter:mainfrom
filipsajdak:fsajdak-fix-short-syntax-lambda-and-contract
Apr 16, 2023
Merged

Correction for short function syntax, closes #356#359
hsutter merged 1 commit intohsutter:mainfrom
filipsajdak:fsajdak-fix-short-syntax-lambda-and-contract

Conversation

@filipsajdak
Copy link
Contributor

@filipsajdak filipsajdak commented Apr 11, 2023

The current implementation does not work for the following code:

main: () = {
    :() = 1;
    [[assert: 1]]
}

It fails with the error:

error: subscript expression [ ] must not be empty (if you were trying to name a C-style array type, use 'std::array' instead) (at '[')

This change introduces a minor correction that moves back parsing to a semicolon (to simulate a double semicolon) for short syntax.

It is not done in the following cases:

:() = 1;(); // immediately called lambda
f(a,b,:() = 1;); // last argument in function call
f(a,:() = 1;,c); // first or in the middle argument

After this change, the original issue is solved.
All regression tests pass. Closes #356

@filipsajdak filipsajdak mentioned this pull request Apr 11, 2023
Closed
@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 11, 2023

I can confirm that the original issue is solved with this. Thank you.

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 12, 2023 •
edited
Loading

Let me add that I'm not sure of the implications of this solution. I hadn't thought of IIFE for simple function expressions.

I tested the following on branch main. The handling of IIFEs is inconsistent. Is that because it is a semantic exception that applies to a function scope? In particular, for main and the type, they are all in a statement (as opposed to the declaration-seq of the top-level). See https://godbolt.org/z/vYsqTe3Gq.

Cpp2 code demonstrating the issue. 
a0: std::any = :() -> _ = { return 0; }();

// // error: unexpected end of source file
// b0: std::any = :() -> _ = { return 0; }(
// );

// // error: unexpected end of source file
// c0: std::any = :() -> _ = { return 0; }
// ();

d0: std::any = :() -> _ = 0;();

// // error: unexpected end of source file
// e0: std::any = :() -> _ = 0;(
// );

f0: std::any = :() -> _ = 0;
(); // mixed2.cpp2:18:2: error: expected unqualified-id

main: () = {
  a1: std::any = :() -> _ = { return 0; }();

  b1: std::any = :() -> _ = { return 0; }(
  );

  c1: std::any = :() -> _ = { return 0; }
  ();

  d1: std::any = :() -> _ = 0;();

  e1: std::any = :() -> _ = 0;(
  );

  f1: std::any = :() -> _ = 0;
  ();
}

x: type = {
  // Enabled individually, all generate:
  // ```C++
  //   private: std::any 𝘓2 {[]() -> auto;
  // ()}; public: x() = default; x(x const&) = delete; auto operator=(x const&) -> void = delete;
  // ```

  // a2: std::any = :() -> _ = { return 0; }();

  // b2: std::any = :() -> _ = { return 0; }(
  // );

  // c2: std::any = :() -> _ = { return 0; }
  // ();

  // d2: std::any = :() -> _ = 0;();

  // e2: std::any = :() -> _ = 0;(
  // );

  // f2: std::any = :() -> _ = 0;
  // ();
}

Should I open an issue for this?

@filipsajdak
Copy link
Contributor Author

filipsajdak commented Apr 12, 2023

No need for a new issue as this is not yet merged.

@filipsajdak
Copy link
Contributor Author

filipsajdak commented Apr 12, 2023

For the following code:

c0: std::any = :() -> _ = { return 0; }
();

first line is cpp2, the second cpp1:

+/* 2 */ c0: std::any = :() -> _ = { return 0; }
+/* 1 */ ();

Nevertheless it should just inform that there is an issue with missing semicolon.

@filipsajdak
Copy link
Contributor Author

filipsajdak commented Apr 12, 2023

I have pushed the fix - the issue is with the error() method used to report errors.
The method uses curr() internally, which throws when cppfront has processed all tokens.

cppfront/source/parse.h

Line 2760 in f83ca9b

m += std::string(" (at '") + curr().to_string(true) + "')";

cppfront/source/parse.h

Lines 2787 to 2795 in f83ca9b

auto curr() const
-> token const&
{
if (done()) {
throw std::runtime_error("unexpected end of source file");
}
return (*tokens_)[pos];
}

@filipsajdak
Copy link
Contributor Author

filipsajdak commented Apr 12, 2023

Now the error looks like the following:

bug_352_error_message_semicolon.cpp2...
bug_352_error_message_semicolon.cpp2(1,39): error: ill-formed initializer (after '}')
bug_352_error_message_semicolon.cpp2(1,1): error: unexpected text at end of Cpp2 code section (at 'c0')
bug_352_error_message_semicolon.cpp2(1,0): error: parse failed for section starting here

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 12, 2023

Yes, with the build from 2b730e8.

@filipsajdak
Correction for short function syntax, closes hsutter#356
d737f9e 
Current implementation does not work for the following code:
```cpp
main: () = {
    :() = 1;
    [[assert: 1]]
}
```
It fails with error:
```
error: subscript expression [ ] must not be empty (if you were trying to name a C-style array type, use 'std::array' instead) (at '[')
```

This change introduce small correction that moves back parsing
to semicolon (to simulate double semicolon) for short syntax.

It is not done in the following cases:
```cpp
:() = 1;(); // imediatelly called lambda
f(a,b,:() = 1;); // last argument in function call
f(a,:() = 1;,c); // first or in the middle argument
```

After this change the original issue is solved.
All regression tests pass. Closes hsutter#356
@filipsajdak filipsajdak force-pushed the fsajdak-fix-short-syntax-lambda-and-contract branch from 4b3283f to d737f9e Compare April 16, 2023 00:00
@hsutter hsutter merged commit 19132f1 into hsutter:main Apr 16, 2023
@hsutter
Copy link
Owner

hsutter commented Apr 16, 2023

Looks good, thanks!

@filipsajdak filipsajdak deleted the fsajdak-fix-short-syntax-lambda-and-contract branch April 16, 2023 00:15
@JohelEGP JohelEGP mentioned this pull request Nov 1, 2023
Merged
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
@filipsajdak @zaucy
Correction for short function syntax, closes hsutter#356 (hsutter#359)
abb61ba 
Current implementation does not work for the following code:
```cpp
main: () = {
    :() = 1;
    [[assert: 1]]
}
```
It fails with error:
```
error: subscript expression [ ] must not be empty (if you were trying to name a C-style array type, use 'std::array' instead) (at '[')
```

This change introduce small correction that moves back parsing
to semicolon (to simulate double semicolon) for short syntax.

It is not done in the following cases:
```cpp
:() = 1;(); // imediatelly called lambda
f(a,b,:() = 1;); // last argument in function call
f(a,:() = 1;,c); // first or in the middle argument
```

After this change the original issue is solved.
All regression tests pass. Closes hsutter#356
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Contract after simple expression function fails

3 participants

@filipsajdak @JohelEGP @hsutter